Skip to content

London| 26-ITP-May | Mandip Sanger | Sprint 3 | Alarm Clock - #1435

Open
mandipsanger wants to merge 9 commits into
CodeYourFuture:mainfrom
mandipsanger:Sprint-3
Open

London| 26-ITP-May | Mandip Sanger | Sprint 3 | Alarm Clock#1435
mandipsanger wants to merge 9 commits into
CodeYourFuture:mainfrom
mandipsanger:Sprint-3

Conversation

@mandipsanger

@mandipsanger mandipsanger commented Aug 14, 2026 •
edited
Loading

Copy link
Copy Markdown

Self checklist

Sprint 3 AlarmClock

@github-actions

This comment has been minimized.

Sign in to view
@mandipsanger mandipsanger added 📅 Sprint 3 Assigned during Sprint 3 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 14, 2026
cjyuan
cjyuan reviewed Aug 14, 2026
View reviewed changes

@cjyuan cjyuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • You missed updating index.html according to an instruction in readme.md.

  • Currently when starting a new countdown, the application does not always return to a clean initial state, which can lead to inconsistent behaviour between runs.

    Note: a user may not click the "Stop Alarm" button first before starting a new count down.

Comment thread Sprint-3/alarmclock/alarmclock.js Outdated
@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 14, 2026
@mandipsanger mandipsanger added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Aug 18, 2026
cjyuan
cjyuan reviewed Aug 18, 2026
View reviewed changes
Comment thread Sprint-3/alarmclock/index.html
<label for="alarmSet">Set time to:</label>
<input id="alarmSet" type="number" />
<label for="alarmSet">Set time in seconds:</label>
<input id="alarmSet" type="number" min="1" step="1" placeholder="Enter seconds" />

@cjyuan cjyuan Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Without a form, the browser won't check the input against the constraints min="1" step="1".

Change is optional.

Comment thread Sprint-3/alarmclock/alarmclock.js Outdated
Comment thread Sprint-3/alarmclock/alarmclock.js
@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 18, 2026
cjyuan
cjyuan reviewed Aug 20, 2026
View reviewed changes

@cjyuan cjyuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this PR is ready to be re-reviewed.

Changes look good.

Comment thread Sprint-3/alarmclock/alarmclock.js
Comment on lines 17 to 26
if (!Number.isInteger(numberValue)) {
alert("only enter an integer value.");
return; // exit a function if not an integer
alert("Only enter an integer value.");
return;
}

//step 4 validate if it is greater than 0
// Step 4: validate if it is greater than 0
if (numberValue <= 0) {
alert("only enter a number which is bigger than 0");
return; //Exit a funciton if not greater than 0
alert("Only enter a number which is bigger than 0.");
return;
}

@cjyuan cjyuan Aug 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notes:

  • Could probably just check both conditions in the one if-statement and then change the message to mention "... positive integer".

  • While alert() is convenient to use, it is not a user-friendly way to notify the user an error.

No change needed.

@mandipsanger mandipsanger Aug 20, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes i made changes but it is still showing to do list I have replaced the orignial file and pushed it through, but I think I have made both under sprint 3 so thats why its showing like this. any idea how can i sort it out usually when I replace the original file from github it goes away but this time it didnt. May be becuase they both in sprint 3. Are you able to help otherwise its failing the checks

@cjyuan cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Aug 20, 2026
@github-actions

This comment has been minimized.

Sign in to view
1 similar comment
@github-actions

This comment has been minimized.

Sign in to view
@cjyuan

cjyuan commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

In your most recent commit, you introduced a modified file in the todo-list folder. Could your revert the change to that file to keep this PR clean?

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Complete Volunteer to add when work is complete and all review comments have been addressed. labels Aug 20, 2026
@github-actions

This comment has been minimized.

Sign in to view
1 similar comment
@github-actions

This comment has been minimized.

Sign in to view
@mandipsanger mandipsanger added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 20, 2026
@github-actions

This comment has been minimized.

Sign in to view
@github-actions github-actions Bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 20, 2026
@cjyuan

cjyuan commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

One way to revert changes to a file is via the git restore command.

First, locate a commit before the file was modified. It could be the first commit you made, assuming the branch started in a clean state.

Next, record the first 7 characters of the commit SHA.

Suppose the file is path/to/file, and you want to restore the file to its state before the commit with SHA 1234567.
The command is:

git restore --source=1234567^ path/to/file

After the file is restored, make a commit and push the changes to GitHub.

@mandipsanger mandipsanger added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Aug 21, 2026
@cjyuan

cjyuan commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

You fixed the PR branch. Well done. All good now.

@cjyuan cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 21, 2026
@mandipsanger mandipsanger mentioned this pull request Aug 21, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cjyuan cjyuan cjyuan left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. 📅 Sprint 3 Assigned during Sprint 3 of this module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mandipsanger @cjyuan